Skip to content

[mypyc] feat: new primitive for int.bit_length #19673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

[mypyc] feat: new primitive for int.bit_length #19673

wants to merge 32 commits into from

Conversation

BobTheBuidler
Copy link
Contributor

@BobTheBuidler BobTheBuidler commented Aug 16, 2025 •
edited
Loading

This PR adds a new primitive for int.bit_length and is already ready for review.

BobTheBuidler
BobTheBuidler commented Aug 16, 2025
View reviewed changes
BobTheBuidler
BobTheBuidler commented Aug 16, 2025
View reviewed changes
JukkaL
JukkaL reviewed Aug 18, 2025
View reviewed changes
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! bit_length can be useful in low-level code, where a generic method call can really slow things down.

@BobTheBuidler BobTheBuidler requested a review from JukkaL August 20, 2025 21:49
@BobTheBuidler BobTheBuidler mentioned this pull request Aug 21, 2025
Open
@BobTheBuidler
Copy link
Contributor Author

I'm not entirely sure how to proceed here, I can't think of any reason our use of getattr would lead to segfault behavior where there was none before

@BobTheBuidler
Copy link
Contributor Author

@JukkaL This one I could definitely use some help with though. I have no idea why 1eb2c7e might cause segfault behavior and I'm inclined to say maybe it's a non-issue and we can mark this test as 64-bit only?

JukkaL
JukkaL reviewed Sep 9, 2025
View reviewed changes
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found an issue. Also can you measure the perf difference using a micro-benchmark?

@BobTheBuidler
Copy link
Contributor Author

BobTheBuidler commented Sep 9, 2025 •
edited
Loading

Also can you measure the perf difference using a micro-benchmark?

Will this benchmark suffice? I can deploy this commit hash and compare against 1.18 which I already have benchmarked here

@BobTheBuidler
Copy link
Contributor Author

Also can you measure the perf difference using a micro-benchmark?

Will this benchmark suffice? I can deploy this commit hash and compare against 1.18 which I already have benchmarked here

1.18 vs this branch

There was a ~6% increase in speed for various permutations of the int_to_big_endian benchmark which includes not only a call to bit_length but a decent bit of other stuff as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JukkaL JukkaL JukkaL left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BobTheBuidler @JukkaL